where is the js? :)

On Thu, Oct 3, 2013 at 8:59 PM, Greg Sutcliffe notifications@github.comwrote:

#3182
#3196

We might want some kind of AJAX on the IP field to display when it
is/isn't required. Otherwise the user may get confused. Conditions are:

• When it's on a DNS or DHCP enabled Subnet
• When it's on a DNS enable Domain
• When Tokens are disabled
• Not when it's an image-based Compute VM

---

You can merge this Pull Request by running

git pull https://github.com/GregSutcliffe/foreman 3182-ip

Or view, comment on, or merge it at:

#927
Commit Summary

• Fixes fixes #13409 - remove early initialisation of test DB #3182 - Make IP optional in some circumstances
• Fixes Fixes #13753 - adding statistics on compute_resources usage #3196 - Switch tokens to default to on

File Changes

• M app/models/host/managed.rbhttps://github.com/IPs and tokens update #927/files#diff-0(9)
• M app/models/setting/provisioning.rbhttps://github.com/IPs and tokens update #927/files#diff-1(2)
• M test/fixtures/subnets.ymlhttps://github.com/IPs and tokens update #927/files#diff-2(9)
• M test/unit/host_test.rbhttps://github.com/IPs and tokens update #927/files#diff-3(76)

Patch Links:

• https://github.com/theforeman/foreman/pull/927.patch
• https://github.com/theforeman/foreman/pull/927.diff

I said we might need to do that :P

I do think the AJAX idea would be good, and can't really see how we could do it with pure client side validation. Would it though cause more calls to the CR, similar to #936, as we'd need the whole host?

There's already a CR selected route which returns some rendered HTML, perhaps that could be changed to a JSON response and we include the validation state alongside. Maybe an additional AJAX call just for validation triggered by domain+subnet selection would complete it?

I wonder how much additional load this would put on Foreman. Any better ideas @ohadlevy?

I guess we could split up the validation. Client-side, we only need to know if the user needs to supply an IP. So we can split this into "require_ip_input?" and "require_ip_validation?". The first method won't involve calls to the CR, and the second will only need to be checked on save. We can then use the first as a base for some AJAX.

I think the two are very much linked - even knowing if an IP is required will need us to know which CR it is. Most of the AJAX calls work by serialising the entire form, which causes the CR to be instantiated. I'm not sure if it calls the CR for anything though - I got the impression from @ohadlevy's comments the other day that this process does.

bump, this is becoming important. Do we merge as is, or can someone help me get it where it needs to be?

Does the AJAX solution outlined in #927 (comment) make sense?

I think so, although I've never tried to write any ajaxy stuff. I guess I can try to see how the primary tab does it right now, and do some googling

Have a look at what's there already for CR selection and domain selection - the former returns an HTML view that I think we'd change to JSON so we can pass the IP validation info alongside, while the latter is probably JSON that returns a list of subnets.

Having played around with this for a while, I think it's a huge amount of overhead to calculate require_ip_validation? as an AJAX call - it would involve serialising the host repeatedly and updating several controller methods to parse the whole host object. Definitely overkill

I suggest we had a hover tooltip explaining when an IP is/is not required - is that ok?

@GregSutcliffe I'm not sure 100% by what you mean is a huge amount of overhead... can you break it down?

To decide if we require an IP or not, we need to know:

• domain_id
• subnet_id
• compute_resource_id

We also need to evaluate this when there are changes to any of these parameters.

If we look at the domain_selected method, called when the user selects a domain, we currently only send the domain_id (and organization_id/location_id). That's not enough information to calculate the IP validation rules. In addition require_ip_validation? is currently an instance method, not a class method. So we would have to:

• Update all the JS for these elements to send all the required information each time
• Update the controller methods (such as domain_selected) to:
  • Instantiate a Host object and calculate require_ip_validation?
  • Return true/false in the JSON to indicate the requirement
• Update the JS in the form to change the text based on the response.

That's a huge amount of complication for changing some text. In addition, I already have timeout issues around the compute resource parts of the Host form - this will make it worse, I think.

A popup helpbox is much easier.

@domcleal i've just pushed a simple popover that explains when you can ignore the IP box. I've not mentioned CRs since the IP box vanishes entirely then.

Merged as 01302dc and 385d994, thanks @GregSutcliffe.

One issue I hit while testing is that spoof stops working, as the IP is no longer present. I've bumped issue 359 up the backlog in response, particularly as it already has a patch. If you could look at it in the next sprint and add tests to @jfautley's patch, that'd be appreciated.